Skip to content

change the default cluster naming#8222

Open
zirain wants to merge 3 commits intoenvoyproxy:mainfrom
zirain:always-WeightedCluster
Open

change the default cluster naming#8222
zirain wants to merge 3 commits intoenvoyproxy:mainfrom
zirain:always-WeightedCluster

Conversation

@zirain
Copy link
Member

@zirain zirain commented Feb 9, 2026

This's a little tricky, but will fix #6287.
This patch change the cluster naming to make the cluster name be same w/wo backend filters.

@netlify
Copy link

netlify bot commented Feb 9, 2026

Deploy Preview for cerulean-figolla-1f9435 ready!

Name Link
🔨 Latest commit 7a718fc
🔍 Latest deploy log https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/699517cd2302d50008162db8
😎 Deploy Preview https://deploy-preview-8222--cerulean-figolla-1f9435.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@zirain zirain force-pushed the always-WeightedCluster branch 3 times, most recently from b561638 to f4f38e8 Compare February 9, 2026 07:41
@codecov
Copy link

codecov bot commented Feb 9, 2026

Codecov Report

❌ Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.57%. Comparing base (e56785c) to head (7a718fc).

Files with missing lines Patch % Lines
internal/gatewayapi/route.go 83.33% 1 Missing ⚠️
internal/gatewayapi/runner/runner.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8222      +/-   ##
==========================================
- Coverage   73.57%   73.57%   -0.01%     
==========================================
  Files         242      242              
  Lines       37000    37004       +4     
==========================================
+ Hits        27224    27225       +1     
- Misses       7854     7856       +2     
- Partials     1922     1923       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@zirain zirain force-pushed the always-WeightedCluster branch 2 times, most recently from 4273702 to a927176 Compare February 9, 2026 13:06
@zirain zirain changed the title always use WeightedCluster change the default cluster naming Feb 9, 2026
@zirain zirain force-pushed the always-WeightedCluster branch 5 times, most recently from 3311249 to 4e6b6cd Compare February 12, 2026 06:17
@zirain zirain marked this pull request as ready for review February 12, 2026 11:38
@zirain zirain requested a review from a team as a code owner February 12, 2026 11:38
@arkodg
Copy link
Contributor

arkodg commented Feb 13, 2026

what will be the cluster name for the non backend filter case when there are N backendRefs

@zirain
Copy link
Member Author

zirain commented Feb 14, 2026

what will be the cluster name for the non backend filter case when there are N backendRefs

change from httproute/default/httproute/rule/0 to httproute/default/httproute/rule/0/backend/0

@zirain zirain force-pushed the always-WeightedCluster branch 2 times, most recently from 9a084da to 661283f Compare February 14, 2026 01:13
@zirain zirain requested review from arkodg and jukie February 14, 2026 01:13
@zirain zirain force-pushed the always-WeightedCluster branch from 661283f to dd1b4d8 Compare February 14, 2026 01:41
@arkodg
Copy link
Contributor

arkodg commented Feb 14, 2026

what does the 0 in backend/0 map to if there are N backends

@zirain
Copy link
Member Author

zirain commented Feb 14, 2026

what does the 0 in backend/0 map to if there are N backends

Nothing

If need N clusters, you will got backend/1.

kind: HTTPRoute
name: httproute-1
namespace: default
name: httproute/default/httproute-1/rule/0/backend/0
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arkodg this's the line which changed from httproute/default/httproute-1/rule/0

@arkodg
Copy link
Contributor

arkodg commented Feb 14, 2026

what does the 0 in backend/0 map to if there are N backends

Nothing

If need N clusters, you will got backend/1.

can you elaborate what xDS resources will be needed

  • route
  • weighted cluster
  • cluster

today, for this case, we dont use any weighted cluster for this case

for this case

  rules:
  - matches:
    - path:
        type: PathPrefix
        value: /login
    backendRefs:
    - name: foo-svc-1
      port: 8080
     - name: foo-svc-2
      port: 8080
     - name: foo-svc-3
      port: 8080

@zirain
Copy link
Member Author

zirain commented Feb 15, 2026

what does the 0 in backend/0 map to if there are N backends

Nothing
If need N clusters, you will got backend/1.

can you elaborate what xDS resources will be needed

  • route
  • weighted cluster
  • cluster

today, for this case, we dont use any weighted cluster for this case

for this case

  rules:
  - matches:
    - path:
        type: PathPrefix
        value: /login
    backendRefs:
    - name: foo-svc-1
      port: 8080
     - name: foo-svc-2
      port: 8080
     - name: foo-svc-3
      port: 8080

yeah, today you will have a cluster named httproute/ns/name/rule/0.
After you add filters to the backend, you will got three new clusters:

  • httproute/ns/name/rule/0/backend/0
  • httproute/ns/name/rule/0/backend/1
  • httproute/ns/name/rule/0/backend/2

With this patch, httproute/ns/name/rule/0/backend/0 will always there to avoid NC when CP updateing the XDS.

@arkodg
Copy link
Contributor

arkodg commented Feb 16, 2026

httproute/ns/name/rule/0/backend/0

-1 on special naming only for 0th index

the field name here impacts

  • triaging
  • tooling ingesting config dump
  • metrics

and this will add unnecessary complexity and cognitive load for users

Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
@zirain zirain force-pushed the always-WeightedCluster branch from 4838bd7 to 7a718fc Compare February 18, 2026 01:37
@zirain
Copy link
Member Author

zirain commented Feb 18, 2026

httproute/ns/name/rule/0/backend/0

-1 on special naming only for 0th index

the field name here impacts

  • triaging
  • tooling ingesting config dump
  • metrics

and this will add unnecessary complexity and cognitive load for users

I understand this's an edge case, could we do something to reduce the noise? what about add a runtime flag for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Received 500/503 when changing the backendRef in HTTPRoute

3 participants